Add Support For files.getUploadURLExternal & files.completeUploadExternal #182
Conversation
damienalexandre
left a comment
damienalexandre
left a comment
There was a problem hiding this comment.
Thanks a lot for this implementation and the new helper function in the client ! I just have one question.
src/Client.php
Outdated
| ); | ||
| } | ||
|
|
||
| private function uploadToSlackUrlWithCurl(string $uploadUrl, string $filePath): void |
There was a problem hiding this comment.
It would be nice to use the http client user in the actual api client (Jane).
Would that be possible?
There was a problem hiding this comment.
I tried it out but couldn't make it work. Ended up moving from cURL and using GuzzleClient instead. Let me know if this is not ok for you.
I got following error with generated jane api client:
1) JoliCode\Slack\Tests\WritingTest::testItCanUploadFilesViaFilesUploadV2
Http\Client\Common\Exception\ClientErrorException: Not Found
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/Plugin/ErrorPlugin.php:80
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/Plugin/ErrorPlugin.php:62
/Users/john.doe/repositories/slack-php-api/vendor/php-http/httplug/src/Promise/HttpFulfilledPromise.php:28
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/Plugin/ErrorPlugin.php:63
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/PluginChain.php:44
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/PluginChain.php:59
/Users/john.doe/repositories/slack-php-api/vendor/php-http/client-common/src/PluginClient.php:84
/Users/john.doe/repositories/slack-php-api/src/Client.php:193
/Users/john.doe/repositories/slack-php-api/src/Client.php:151
/Users/john.doe/repositories/slack-php-api/tests/WritingTest.php:264
There was a problem hiding this comment.
That was discussed here too: #176
I like what you did yes 👍 much more closer to the actual API client that way. I will suggest a modification tho.
src/Client.php
Outdated
| $client = Psr18ClientDiscovery::find(); | ||
| $response = $client->sendRequest($request); |
There was a problem hiding this comment.
We must use the Client provided by the developer, so I think this should be:
| $client = Psr18ClientDiscovery::find(); | |
| $response = $client->sendRequest($request); | |
| $response = $this->httpClient->sendRequest($request); |
The HttpClient is from here:
There was a problem hiding this comment.
That's exactly what I did, it doesn't work at least when I run my test :(
There was a problem hiding this comment.
@damienalexandre it doesn't work because generated client has base uri set as slack.com and what getUploadUrlExternal returns is a different url i.e., https://files.slack.com/upload/v1/CwABAAAAYwoAAdZbBYEretOHCgACGDH-HcmojZAMAAMLAAEAAAALRTA3UEI5R1QzTU0LAAIAAAALVTA4S1YyNjkwOEsLAAMAAAALRjA4S1kyQVRFUTYACgAEAAAAAAAAAJoIAAcAAAAEAAsAAgAAABTBhf2hsqHZcysRfVuQ1zXp61dVZAA and that's why I am getting 404s
There was a problem hiding this comment.
Ok I see, I just tested your branch, and found a way to keep the "files.slack.com" URI when using the internal HTTP Client. I just pushed a new commit in your PR to add this. Is it ok for you?
There was a problem hiding this comment.
Nice, looks good! 👍
|
Thanks a lot! This was an highly requested feature! 👏 |
2 participants
Slack deprecated https://api.slack.com/methods/files.upload and introduced a new flow for file uploads, which comprises of the following steps:
Fixes #168